Conversation
📝 WalkthroughWalkthroughThis PR refactors session management authentication, adds username to profile API responses, synchronizes theming with next-themes in the settings UI, moves SettingsTab to a centralized type definition, and applies border styling updates across UI primitives. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary by CodeReverb
Generated automatically by CodeReverb Try out CodeReverb |
Code review by CodeReverbSummaryThis pull request introduces significant changes to the settings pages and their associated API routes, primarily focusing on session management and profile updates. It also includes type definition enhancements and minor UI styling adjustments.
Changes (per file)File:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Fix all issues with AI Agents 🤖
In @app/(dashboard)/settings/page.tsx:
- Around line 27-31: The current @ts-ignore hides a type mismatch between
params.settings_tab and the SettingsSidebar/SettingsContent props; fix by
aligning types instead of ignoring: update your nuqs schema to parse
settings_tab as the SettingsTab enum (e.g., in nuqs/index.ts use
parseAsStringEnum<SettingsTab>(...) with a default) so params.settings_tab is
typed correctly, or at the usage site replace the @ts-ignore with an explicit
assertion activeSection={params.settings_tab as SettingsTab}; remove the
@ts-ignore lines and ensure both SettingsSidebar and SettingsContent accept
SettingsTab typed values.
In @app/api/settings/sessions/[sessionId]/route.ts:
- Around line 21-31: Check that prisma.session.findUnique returns a session and
that it belongs to the authenticated user before proceeding: modify the
findUnique call (symbol: prisma.session.findUnique) to include userId:
session.user.id in the where clause, then if sessionToRevoke is null return
NextResponse.json({ error: "Session not found" }, { status: 404 }); only after
that compare sessionToRevoke.token with currentToken and proceed with deletion;
ensure you reference sessionToRevoke and currentToken exactly as in the diff.
In @components/settings/settings-content.tsx:
- Around line 326-327: The githubUrl fallback builds
`https://github.com/${profile.username}` without guarding against an undefined
`profile.username`; update the `githubUrl` assignment (the `githubUrl` property
where `profile?.githubUrl || \`https://github.com/${profile.username}\` || ""`
is used) to only construct the URL when `profile?.username` exists (e.g. use a
conditional or nullish-coalescing: `profile?.githubUrl ?? (profile?.username ?
\`https://github.com/${profile.username}\` : "")`) so you don't interpolate
`undefined` into the URL.
- Around line 358-363: The useEffect should guard against
appearanceSettings.theme being undefined and include setTheme in the dependency
array; inside the effect call setAppearanceData(appearanceSettings) as before
but only call setTheme with a defined value (e.g., appearanceSettings.theme ??
<defaultTheme>) or check if appearanceSettings.theme !== undefined before
calling setTheme, and add setTheme to the dependency array so the effect reads:
useEffect(() => { if (appearanceSettings) {
setAppearanceData(appearanceSettings); if (appearanceSettings.theme !==
undefined) setTheme(appearanceSettings.theme); } }, [appearanceSettings,
setTheme]);
- Around line 258-259: The githubUrl assignment (githubUrl in
settings-content.tsx) currently uses profile?.githubUrl ||
`https://github.com/${profile.username}` || "" which is redundant and can
produce "https://github.com/undefined"; change it to prefer profile?.githubUrl,
then only build the template URL if profile?.username is present, otherwise fall
back to an empty string (e.g., use nullish/conditional logic so the template
literal is only used when profile.username exists).
In @nuqs/index.ts:
- Around line 21-23: Fix the typo in the settings tab list: replace the
misspelled string "secutiry" with "security" in the exported tab
enumeration/array defined in index.ts (the array containing "account",
"secutiry", "repositories") so URL param ?settings_tab=security will match.
Ensure the corrected string is used wherever that exported list (settings tabs
enum/array) is referenced for routing or URL parsing.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/api/settings/sessions/route.ts (1)
119-140: Missing existence check for the session to revoke.If
sessionIdrefers to a non-existent session,sessionToRevokewill benull. The optional chaining at line 126 will evaluate toundefined !== currentToken, passing the check. The subsequentprisma.session.deletewill throw an error for a non-existent record, resulting in a generic 500 error instead of a proper 404.🔎 Proposed fix
const sessionToRevoke = await prisma.session.findUnique({ where: { id: sessionId }, }); + if (!sessionToRevoke) { + return NextResponse.json( + { error: "Session not found" }, + { status: 404 } + ); + } + // Prevent revoking current session - if (sessionToRevoke?.token === currentToken) { + if (sessionToRevoke.token === currentToken) { return NextResponse.json( { error: "Cannot revoke current session" }, { status: 400 } ); }app/api/settings/profile/route.ts (1)
56-67: Inconsistent response shape: PUT handler missing username field.The GET handler now returns
username(line 22), but the PUT handler's select statement doesn't include it. This creates inconsistent response shapes between GET and PUT, which could break frontend code expecting a uniformUserProfiletype.🔎 Proposed fix to include username in PUT response
select: { id: true, name: true, email: true, bio: true, location: true, website: true, githubUrl: true, image: true, showEmail: true, showLocation: true, + username: true, },
🧹 Nitpick comments (2)
app/api/settings/sessions/route.ts (1)
1-6: Remove unused importrequireAuth.The
requireAuthimport on line 2 is no longer used after refactoring toauth.api.getSession. This is dead code.🔎 Proposed fix
import { NextRequest, NextResponse } from "next/server"; -import { requireAuth } from "@/lib/auth-utils"; import { prisma } from "@/lib/prisma"; import { UAParser } from "ua-parser-js"; import { auth } from "@/lib/auth"; import { headers } from "next/headers";components/settings/settings-sidebar.tsx (1)
144-149: Consider consolidating active state styling.The active state is handled in two places: via
variantprop and viaclassNamewith!importantoverrides. This duplication can lead to maintenance issues.If the
secondaryvariant doesn't provide the desired styling, consider either:
- Customizing the button variant in your design system, or
- Removing the variant logic and relying solely on
cn()classes.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/(dashboard)/settings/page.tsxapp/api/settings/profile/route.tsapp/api/settings/sessions/[sessionId]/route.tsapp/api/settings/sessions/route.tscomponents/settings/settings-content.tsxcomponents/settings/settings-sidebar.tsxcomponents/ui/input.tsxcomponents/ui/switch.tsxcomponents/ui/textarea.tsxlib/api-client.tsnuqs/index.tstypes.d.ts
🧰 Additional context used
🧬 Code graph analysis (5)
app/api/settings/sessions/route.ts (1)
lib/auth.ts (1)
auth(6-24)
components/ui/textarea.tsx (1)
lib/utils.ts (1)
cn(4-6)
components/settings/settings-content.tsx (3)
components/ui/card.tsx (1)
CardContent(91-91)components/ui/dialog.tsx (3)
DialogDescription(136-136)DialogHeader(138-138)DialogFooter(137-137)components/ui/button.tsx (1)
Button(59-59)
app/api/settings/sessions/[sessionId]/route.ts (2)
app/api/settings/sessions/route.ts (1)
DELETE(104-161)lib/auth.ts (1)
auth(6-24)
components/ui/switch.tsx (1)
lib/utils.ts (1)
cn(4-6)
🔇 Additional comments (15)
app/api/settings/sessions/route.ts (2)
62-72: LGTM!The refactored authentication using
auth.api.getSessionwith proper 401 handling is correct and follows a consistent pattern.
104-114: LGTM!The DELETE handler's authentication refactoring is consistent with the GET handler and properly returns 401 for unauthorized requests.
app/api/settings/sessions/[sessionId]/route.ts (2)
6-8: LGTM!The
paramstype correctly usesPromise<{ sessionId: string }>which is required for Next.js 15+ async route parameters.
11-18: LGTM!Consistent authentication pattern with proper 401 handling.
app/api/settings/profile/route.ts (1)
22-22: LGTM! Username field added to profile response.The addition of the
usernamefield aligns with the newUserProfiletype definition and enriches the profile API response.components/ui/switch.tsx (1)
16-16: LGTM! Consistent border styling applied.The
border-border!class additions align with the broader UI styling updates across components, ensuring consistent border theming.Also applies to: 24-24
types.d.ts (2)
31-42: LGTM! Well-structured UserProfile interface.The
UserProfileinterface correctly captures all profile fields returned by the API, including the newly addedusername. The optionalimagefield is appropriately typed.
44-53: LGTM! Type-safe settings navigation.The
SettingsTabunion type provides compile-time safety for settings navigation and centralizes the valid tab values.components/ui/textarea.tsx (1)
10-10: LGTM! Consistent border styling applied.The
border-border!class addition aligns with the UI-wide border theming updates.components/ui/input.tsx (1)
13-13: LGTM! Consistent border styling applied.The
border-border!class addition maintains consistent border theming across input components, even though it's grouped with thearia-invalidclasses for organization.components/settings/settings-sidebar.tsx (1)
18-18: LGTM!Good addition of the
cnutility for conditional class merging.components/settings/settings-content.tsx (3)
63-63: Good integration with next-themes.The pattern of calling
setThemeboth on initial load (from server settings) and on user selection provides immediate visual feedback while persisting the preference.Also applies to: 76-76, 1132-1132
971-971: Good defensive coding with optional chaining.The
aiSettingsData?.maxTokens?.toString()guards against undefined values during initial render before data loads.
66-66: LGTM — improved type safety.Changing
activeSectionfromstringtoSettingsTabprovides better type checking and IDE support.lib/api-client.ts (1)
67-68: Good improvement to type safety.The explicit
UserProfilereturn type is better thanany.UserProfileis properly declared as a global ambient type intypes.d.ts(lines 31-42), making it correctly accessible without explicit import in this file.
| // @ts-ignore | ||
| activeSection={params.settings_tab} | ||
| setActiveSection={(v) => | ||
| // @ts-ignore | ||
| setParams({ ...params, settings_tab: v }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid @ts-ignore — fix the underlying type mismatch instead.
Using @ts-ignore suppresses TypeScript errors without resolving them, which defeats the purpose of having typed props. The mismatch likely stems from params.settings_tab being typed as string (from nuqs) while SettingsSidebar and SettingsContent expect SettingsTab.
Consider typing the nuqs schema to align with SettingsTab:
// In nuqs/index.ts, import and use SettingsTab for better type inference
settings_tab: parseAsStringEnum<SettingsTab>([...]).withDefault("profile")Or cast at the usage site with a type assertion rather than ignoring:
activeSection={params.settings_tab as SettingsTab}Also applies to: 39-40
🤖 Prompt for AI Agents
In @app/(dashboard)/settings/page.tsx around lines 27-31, The current @ts-ignore
hides a type mismatch between params.settings_tab and the
SettingsSidebar/SettingsContent props; fix by aligning types instead of
ignoring: update your nuqs schema to parse settings_tab as the SettingsTab enum
(e.g., in nuqs/index.ts use parseAsStringEnum<SettingsTab>(...) with a default)
so params.settings_tab is typed correctly, or at the usage site replace the
@ts-ignore with an explicit assertion activeSection={params.settings_tab as
SettingsTab}; remove the @ts-ignore lines and ensure both SettingsSidebar and
SettingsContent accept SettingsTab typed values.
| const sessionToRevoke = await prisma.session.findUnique({ | ||
| where: { id: sessionId }, | ||
| }); | ||
|
|
||
| // Prevent revoking current session | ||
| if (sessionToRevoke?.token === currentToken) { | ||
| return NextResponse.json( | ||
| { error: "Cannot revoke current session" }, | ||
| { status: 400 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Missing existence check for the session to revoke.
Same issue as in app/api/settings/sessions/route.ts: if the session doesn't exist, sessionToRevoke is null, and the code proceeds to attempt deletion which will throw, resulting in a 500 error instead of a proper 404.
Additionally, consider adding userId: session.user.id to the findUnique query to ensure the session belongs to the authenticated user before comparing tokens.
🔎 Proposed fix
const { sessionId } = await params;
const sessionToRevoke = await prisma.session.findUnique({
- where: { id: sessionId },
+ where: { id: sessionId, userId: session.user.id },
});
+ if (!sessionToRevoke) {
+ return NextResponse.json(
+ { error: "Session not found" },
+ { status: 404 }
+ );
+ }
+
// Prevent revoking current session
- if (sessionToRevoke?.token === currentToken) {
+ if (sessionToRevoke.token === currentToken) {
return NextResponse.json(
{ error: "Cannot revoke current session" },
{ status: 400 }
);
}
-
- await prisma.session.delete({
- where: {
- id: sessionId,
- userId: session.user.id,
- },
- });
+
+ await prisma.session.delete({
+ where: { id: sessionId },
+ });Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @app/api/settings/sessions/[sessionId]/route.ts around lines 21-31, Check
that prisma.session.findUnique returns a session and that it belongs to the
authenticated user before proceeding: modify the findUnique call (symbol:
prisma.session.findUnique) to include userId: session.user.id in the where
clause, then if sessionToRevoke is null return NextResponse.json({ error:
"Session not found" }, { status: 404 }); only after that compare
sessionToRevoke.token with currentToken and proceed with deletion; ensure you
reference sessionToRevoke and currentToken exactly as in the diff.
| githubUrl: | ||
| profile?.githubUrl || `https://github.com/${profile.username}` || "", |
There was a problem hiding this comment.
Redundant fallback and potential "undefined" in URL.
The trailing || "" is unreachable since template literals always produce a string. Also, if profile.username is undefined/null, this produces "https://github.com/undefined".
🔎 Proposed fix
githubUrl:
- profile?.githubUrl || `https://github.com/${profile.username}` || "",
+ profile?.githubUrl || (profile.username ? `https://github.com/${profile.username}` : ""),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| githubUrl: | |
| profile?.githubUrl || `https://github.com/${profile.username}` || "", | |
| githubUrl: | |
| profile?.githubUrl || (profile.username ? `https://github.com/${profile.username}` : ""), |
🤖 Prompt for AI Agents
In @components/settings/settings-content.tsx around lines 258-259, The githubUrl
assignment (githubUrl in settings-content.tsx) currently uses profile?.githubUrl
|| `https://github.com/${profile.username}` || "" which is redundant and can
produce "https://github.com/undefined"; change it to prefer profile?.githubUrl,
then only build the template URL if profile?.username is present, otherwise fall
back to an empty string (e.g., use nullish/conditional logic so the template
literal is only used when profile.username exists).
| githubUrl: | ||
| profile?.githubUrl || `https://github.com/${profile.username}` || "", |
There was a problem hiding this comment.
Same issue as line 258-259 — guard against undefined username.
🔎 Proposed fix
githubUrl:
- profile?.githubUrl || `https://github.com/${profile.username}` || "",
+ profile?.githubUrl || (profile.username ? `https://github.com/${profile.username}` : ""),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| githubUrl: | |
| profile?.githubUrl || `https://github.com/${profile.username}` || "", | |
| githubUrl: | |
| profile?.githubUrl || (profile.username ? `https://github.com/${profile.username}` : ""), |
🤖 Prompt for AI Agents
In @components/settings/settings-content.tsx around lines 326-327, The githubUrl
fallback builds `https://github.com/${profile.username}` without guarding
against an undefined `profile.username`; update the `githubUrl` assignment (the
`githubUrl` property where `profile?.githubUrl ||
\`https://github.com/${profile.username}\` || ""` is used) to only construct the
URL when `profile?.username` exists (e.g. use a conditional or
nullish-coalescing: `profile?.githubUrl ?? (profile?.username ?
\`https://github.com/${profile.username}\` : "")`) so you don't interpolate
`undefined` into the URL.
| useEffect(() => { | ||
| if (appearanceSettings) { | ||
| setAppearanceData(appearanceSettings); | ||
| setTheme(appearanceSettings?.theme); | ||
| } | ||
| }, [appearanceSettings]); |
There was a problem hiding this comment.
Guard against undefined theme value.
appearanceSettings?.theme could be undefined, which may cause unexpected behavior when passed to setTheme. Additionally, setTheme should be in the dependency array to satisfy exhaustive-deps (though it's stable).
🔎 Proposed fix
useEffect(() => {
if (appearanceSettings) {
setAppearanceData(appearanceSettings);
- setTheme(appearanceSettings?.theme);
+ if (appearanceSettings.theme) {
+ setTheme(appearanceSettings.theme);
+ }
}
- }, [appearanceSettings]);
+ }, [appearanceSettings, setTheme]);🤖 Prompt for AI Agents
In @components/settings/settings-content.tsx around lines 358-363, The useEffect
should guard against appearanceSettings.theme being undefined and include
setTheme in the dependency array; inside the effect call
setAppearanceData(appearanceSettings) as before but only call setTheme with a
defined value (e.g., appearanceSettings.theme ?? <defaultTheme>) or check if
appearanceSettings.theme !== undefined before calling setTheme, and add setTheme
to the dependency array so the effect reads: useEffect(() => { if
(appearanceSettings) { setAppearanceData(appearanceSettings); if
(appearanceSettings.theme !== undefined) setTheme(appearanceSettings.theme); }
}, [appearanceSettings, setTheme]);
| "account", | ||
| "secutiry", | ||
| "repositories", |
There was a problem hiding this comment.
Typo: "secutiry" should be "security".
This typo will break URL-based navigation to the security tab — the URL param ?settings_tab=security won't match the misspelled enum value, causing fallback to the default "profile" tab.
🔎 Proposed fix
"account",
- "secutiry",
+ "security",
"repositories",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "account", | |
| "secutiry", | |
| "repositories", | |
| "account", | |
| "security", | |
| "repositories", |
🤖 Prompt for AI Agents
In @nuqs/index.ts around lines 21-23, Fix the typo in the settings tab list:
replace the misspelled string "secutiry" with "security" in the exported tab
enumeration/array defined in index.ts (the array containing "account",
"secutiry", "repositories") so URL param ?settings_tab=security will match.
Ensure the corrected string is used wherever that exported list (settings tabs
enum/array) is referenced for routing or URL parsing.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
UI Improvements
✏️ Tip: You can customize this high-level summary in your review settings.